Skip to content

Conversation

@Gawor270
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings January 28, 2026 14:03
@linear
Copy link

linear bot commented Jan 28, 2026

@Gawor270 Gawor270 changed the title check game players count by asking fishjam check game players count by requesting fishjam Jan 28, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the room capacity check to query Fishjam (the media server) for the authoritative player count instead of relying on local state. This addresses issues where missed peerDisconnected events could cause the local state to become out of sync with the actual number of connected players.

Changes:

  • Modified addPlayer to call a new getPlayersCount() method that queries Fishjam
  • Added getPlayersCount() method to fetch room info from Fishjam and filter for webrtc peers
  • Added reconcilePlayersCount() method to synchronize local state with Fishjam's state when discrepancies are detected
Comments suppressed due to low confidence (1)

deep-sea-stories/packages/backend/src/game/room.ts:77

  • There is a race condition between checking the player count and creating the peer. If two requests call addPlayer concurrently when the room has ROOM_PLAYERS_LIMIT - 1 players, both could pass the check at line 70 and then both create peers at line 73-75, resulting in a room exceeding the player limit. Consider adding a locking mechanism or moving the player limit check to after peer creation with appropriate cleanup on failure.
		const roomPlayersCount = await this.getPlayersCount();
		if (roomPlayersCount >= ROOM_PLAYERS_LIMIT) {
			throw new GameRoomFullError();
		}
		const { peer, peerToken } = await this.fishjamClient.createPeer(
			this.roomId,
		);

		this.players.set(peer.id, { name });

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Gawor270 Gawor270 merged commit d113540 into main Jan 28, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants